fix(android): release data-binding HybridObject native resources#302
Merged
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Android data-binding HybridObject native resource retention by ensuring dispose() frees both listener resources and the underlying Nitro/HybridObject native resources, and by providing Hermes with a realistic memorySize for view-model wrapper objects so GC triggers under native-pressure churn.
Changes:
- Override
dispose()inHybridViewModel*Propertywrappers that delegateBaseHybridViewModelPropertyso they call bothremoveListeners()andsuper<*Spec>.dispose(). - Add
VIEW_MODEL_HYBRID_MEMORY_SIZEand overridememorySizeon the main view-model wrappers to improve Hermes GC behavior under churn. - Add an example reproducer page to stress view-model property/instance accessors and validate the fix against JNI global-ref overflow.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| example/src/reproducers/AndroidGlobalRefOverflow.tsx | Adds a manual stress reproducer for Android JNI global-ref overflow under view-model accessor churn. |
| android/src/main/java/com/margelo/nitro/rive/BaseHybridViewModelProperty.kt | Introduces a shared VIEW_MODEL_HYBRID_MEMORY_SIZE constant for Hermes GC pressure hinting. |
| android/src/main/java/com/margelo/nitro/rive/HybridViewModelTriggerProperty.kt | Ensures dispose() frees native resources and sets memorySize. |
| android/src/main/java/com/margelo/nitro/rive/HybridViewModelStringProperty.kt | Ensures dispose() frees native resources and sets memorySize. |
| android/src/main/java/com/margelo/nitro/rive/HybridViewModelNumberProperty.kt | Ensures dispose() frees native resources and sets memorySize. |
| android/src/main/java/com/margelo/nitro/rive/HybridViewModelListProperty.kt | Ensures dispose() frees native resources and sets memorySize. |
| android/src/main/java/com/margelo/nitro/rive/HybridViewModelInstance.kt | Sets memorySize for view-model instances to reflect native/JNI cost. |
| android/src/main/java/com/margelo/nitro/rive/HybridViewModelImageProperty.kt | Ensures dispose() frees native resources and sets memorySize. |
| android/src/main/java/com/margelo/nitro/rive/HybridViewModelEnumProperty.kt | Ensures dispose() frees native resources and sets memorySize. |
| android/src/main/java/com/margelo/nitro/rive/HybridViewModelColorProperty.kt | Ensures dispose() frees native resources and sets memorySize. |
| android/src/main/java/com/margelo/nitro/rive/HybridViewModelBooleanProperty.kt | Ensures dispose() frees native resources and sets memorySize. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
mfazekas
force-pushed
the
fix/android-databinding-hybridobject-lifecycle
branch
from
f060b5f to
a08cdc3
Compare
Each HybridViewModel*Property / HybridViewModelInstance is a small JS object that pins a JNI global reference plus a native Rive object. Two problems: - dispose() was hidden: a property both extends its generated *Spec (a HybridObject) and delegates BaseHybridViewModelProperty via 'by'; both declare dispose(), so the delegate's won and hid HybridObject.dispose(). Calling dispose() only cancelled the listener flow, never freeing the native part / global ref. Each property now overrides dispose() to do both. - memorySize was 0: Hermes GCs on JS-heap pressure, so it never collected these tiny-looking but native-heavy wrappers under churn, and the JNI global reference table (cap 51200) overflowed. Report memorySize so the GC collects them in time.
Auto-registers under Reproducers. Churns view-model property/instance accessors (each pins a JNI global ref); without the memorySize fix the Android global reference table (cap 51200) overflows within seconds, with it the counter climbs indefinitely.
… wrappers HybridViewModelArtboardProperty and HybridViewModel also pin a JNI global ref but were left with the default (~0) memorySize, so Hermes could let them accumulate and overflow the global reference table under churn. Give them the same VIEW_MODEL_HYBRID_MEMORY_SIZE hint as the other wrappers.
mfazekas
force-pushed
the
fix/android-databinding-hybridobject-lifecycle
branch
from
a08cdc3 to
a5a8f98
Compare
mfazekas
pushed a commit
that referenced
this pull request
Jul 14, 2026
🤖 I have created a release *beep* *boop* --- ## [0.4.16](v0.4.15...v0.4.16) (2026-07-14) ### Bug Fixes * **android:** release data-binding HybridObject native resources ([#302](#302)) ([4281519](4281519)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Each
HybridViewModel*Property/HybridViewModelInstanceis a small JS object that pins a JNI global reference plus a native Rive object. Two issues with how those were managed:dispose()was hidden. A property both extends its generated*Spec(aHybridObject) and delegatesBaseHybridViewModelPropertyviaby. Both declaredispose(), so the delegate's won and hidHybridObject.dispose()(the "hides supertype override" warning). Callingdispose()only cancelled the listener flow and never freed the native part / global ref. Each property now overridesdispose()to do both (removeListeners()+super<*Spec>.dispose()).memorySizewas 0. Hermes schedules GC on JS-heap pressure, so these tiny-looking but native-heavy wrappers were never collected under churn — the JNI global reference table (cap 51200) filled up and the process aborted withJNI ERROR: global reference table overflow. OverridingmemorySizegives the GC the true cost so it collects them in time.Reproducer: added a toggleable
Android JNI Global-Ref Overflowpage under Reproducers — Start the stress and on an unfixed build the app dies within seconds; with this fix the counter climbs indefinitely. (Same operations as the local harness stress, which aborts every run without the fix and 0 with it.)